Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintainer's guide for Homebrew on Linux (linuxbrew-core) #6448

Merged
merged 1 commit into from
Sep 15, 2019

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Sep 12, 2019

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

  • There are a lot more manual steps in Homebrew on Linux (Linuxbrew)
    land than there are in Homebrew (core), it feels like.
  • This is a first attempt at a maintainer's guide. I have basically
    tried to brain-dump everything that Michka and Shaun have taught me
    over the months into something consumable for future Homebrew on Linux
    maintainers!
  • Some of it is overly verbose - the merging docs are taken from
    Linuxbrew/homebrew-developer's docs
    which were recently updated to reflect reality but probably don't need
    to have so many words: just some instructions. :-)
  • Still to document:
    • the fact that CI is broken on Homebrew/homebrew-core merge PRs
    • how to handle users requesting bottles be built for formulae that don't have them
    • how to pull bottles (brew pull --bottle and brew squash-bottle-pr)
    • merging PRs on GitHub, when do we do that?
    • more that I will doubtless think of later...

Copy link
Contributor

@jonchang jonchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I've made some suggestions in my review.

docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
- there is already an open PR for the formula's bottle
- the current branch is not master

By default, running this script won't output the errors. To see the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like...

If a formula you are expecting to bottle is skipped, there may be an error; by default, this script won't output the errors.

docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
docs/Homebrew-linuxbrew-core-Maintainer-Guide.md Outdated Show resolved Hide resolved
These formulae are fine for users to bump with `brew bump-formula-pr`,
but you should request that they remove the existing `x86_64_linux`
bottle SHA line so that CI will build a bottle for the new version
correctly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify why this is the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
correctly.
correctly. If the bottle SHA isn't removed, CI will fail with the following error:
> `--keep-old` was passed but there are changes in `sha256 => x86_64_linux`


By default, running this script won't output the errors. To see the
errors, run `brew find-formulae-to-bottle --verbose` separate to the
`for` loop above.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably output the errors to stderr by default.

Copy link
Member Author

@issyl0 issyl0 Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll happily review a PR if you find a way to do it. This conversation happened in https://github.com/Linuxbrew/homebrew-developer/pull/112 too (my rationale for --verbose was this and this).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the existing comments but great work here @issyl0. Good to get this all dumped somewhere public. I've found listing things like this is often a good first step to automating things (https://github.blog/2015-10-06-runnable-documentation/ may be an interesting read here).

@@ -0,0 +1,269 @@
# Homebrew on Linux Maintainer Guide
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put linuxbrew-core in the title

The `brew bump-formula-pr` command will raise PRs against the Linux
formula repo for upstream Mac formulae when running on Linux. This
isn't how version bumps are done for _most_ formulae. Redirect users
to raise their PRs in Homebrew/homebrew-core:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could link to #6341 here to nudge people to help?

@MikeMcQuaid
Copy link
Member

Still to document:

  • the fact that CI is broken on Homebrew/homebrew-core merge PRs
  • how to handle users requesting bottles be built for formulae that don't have them
  • how to pull bottles (brew pull --bottle and brew squash-bottle-pr)
  • merging PRs on GitHub, when do we do that?
  • more that I will doubtless think of later...

I think you can feel free to merge this before those are done, if desired. It's sometimes more motivating that way and also easier to review.

@issyl0 issyl0 marked this pull request as ready for review September 14, 2019 11:38
@issyl0 issyl0 changed the title [WIP] First attempt at a maintainer's guide for Homebrew on Linux Maintainer's guide for Homebrew on Linux (linuxbrew-core) Sep 14, 2019
@issyl0
Copy link
Member Author

issyl0 commented Sep 14, 2019

Thanks, @MikeMcQuaid! Merging this as-is would be good, then we can make incremental improvements and document new stuff as we do it. :-)

- There are a lot more manual steps in Homebrew on Linux (Linuxbrew)
  land than there are in Homebrew (core), it feels like.
- This is a first attempt at a maintainer's guide. I have basically
  tried to brain-dump everything that Michka and Shaun have taught me
  over the months into something consumable for future Homebrew on Linux
  maintainers!
- Some of it is overly verbose - the merging docs are taken from
  [Linuxbrew/homebrew-developer's docs](https://github.com/Linuxbrew/homebrew-developer/blob/master/docs/merge-homebrew-core.md)
  which were recently updated to reflect reality but probably don't need
  to have so many words: just some instructions. :-)
@issyl0
Copy link
Member Author

issyl0 commented Sep 14, 2019

Bah, I missed @jonchang's last comment. Amended!

@sjackman sjackman merged commit cae41ef into Homebrew:master Sep 15, 2019
@issyl0 issyl0 deleted the linux-maintainer-guide branch September 16, 2019 22:32
@lock lock bot added the outdated PR was locked due to age label Jan 1, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants